Skip to content

dev: generated checklist of DF defect mitigations for release review - #5941

Open
Alistair-Afton wants to merge 3 commits into
DFHack:developfrom
Alistair-Afton:df-mitigation-checklist
Open

Alistair-Afton wants to merge 3 commits into
DFHack:developfrom
Alistair-Afton:df-mitigation-checklist

Conversation

@Alistair-Afton

Copy link
Copy Markdown
Contributor

Summary

DFHack works around a number of defects in DF itself, and those mitigations may need adjusting or removal when Bay 12 ships a fix. This PR adds a lightweight, self-maintaining checklist for that review:

  • DF-MITIGATION: comment marker convention — mitigations are tagged where they live in the code, so the checklist can't drift away from the implementation. New mitigations get marked as they're added; removing one removes it from the checklist automatically.
  • ci/list-df-mitigations.py — generates a markdown checklist from the markers. The fix/* scripts (and fix-ster/fixnaked) are included automatically since they are all DF bug mitigations by definition; their descriptions come from the docs' :summary: fields.
  • Watch DF Releases workflow — when it detects a new DF build, it now checks out develop (with submodules) and appends the checklist to the job summary. This gives the release coordinator a "soft reinvestigate" signal on every new release, using nothing but free-tier Actions features.
  • Docs — the convention is documented in Contributing, and release-process.rst gained a "New DF releases" section describing the review step.
  • Seed markers on the mitigations I could identify confidently (reclaim site_id fallback, DF bugs 1416/1871/6330/6273/12672/12721, slab category, caged pets, dismissed viewscreens). More can be marked incrementally.

Sample generated output (current tree):

### Code sites
- [ ] `library/modules/Buildings.cpp:131`: monitor works around DF bug 1416 ...
- [ ] `library/modules/World.cpp:221`: reclaimed forts lack site_id until first save (#5716)
- [ ] `plugins/tweak/tweaks/reaction-gloves.h:1`: workaround for DF bug #6273 ...
...
### `fix/*` scripts
- [ ] `scripts/fix/occupancy.lua`: Fix phantom occupancy issues.
- [ ] `scripts/fix/general-strike.lua`: Prevent dwarves from getting stuck and refusing to work.
... (~30 entries)

Open questions for reviewers:

  • Marker name (DF-MITIGATION:) — happy to change to whatever the team prefers.
  • Should the checklist also be posted somewhere more visible than the job summary (e.g. appended to the Discord ping or a pinned issue)?
  • If a mitigation ever becomes automatically testable, the checklist format leaves room to note that per entry.

Test plan

  • ci/list-df-mitigations.py runs locally and produces the checklist (10 code sites + 31 fix scripts)
  • Workflow YAML parses
  • Lint clean on touched Lua files
  • Workflow step itself only executes when a new DF build is detected (guarded by if: env.BUILDID)

DFHack works around a number of defects in DF itself, and those
mitigations need to be re-reviewed whenever Bay 12 releases a new DF
version. Marking them with DF-MITIGATION: comments where they live keeps
the checklist from drifting, and ci/list-df-mitigations.py generates the
review list (the fix/* scripts are included automatically). The Watch DF
Releases workflow appends the checklist to its job summary when it
detects a new release.
Comment thread library/modules/Gui.cpp Outdated
popup->text = message;
popup->color = color; // Doesn't do anything anymore? Popups are always [C:7:0:0] gray text
popup->bright = bright; // See: https://dwarffortressbugtracker.com/view.php?id=12672
// DF-MITIGATION: the bright flag has no effect due to a DF bug; keep it set

@quietust quietust Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is really a mitigation, since what's happening here is that DF ignores the color values we're passing, but we still pass them because eventually it will start respecting them again. Also, this technically applies to the color field too, not just bright (which this new comment incorrectly states).

That being said, once it does get fixed, we'll want to remove these comments, so perhaps this still serves a purpose.

@Alistair-Afton Alistair-Afton Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. folded the pre-existing color comment into the marker so it covers both fields, and updated the convention doc to clarify that markers are also appropriate for sites whose behavior merely depends on a defect (like this one, the point is flagging it for review on new DF releases, whether or not code needs to change). Pushed as 6b401ab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants